Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(core): add aliases to mitigate context issues #7172

Merged
merged 3 commits into from
Jul 19, 2024
Merged

feat(core): add aliases to mitigate context issues #7172

merged 3 commits into from
Jul 19, 2024

Conversation

ricokahler
Copy link
Contributor

@ricokahler ricokahler commented Jul 16, 2024 •
edited
Loading

Description

This pull request enhances the sanity React library's build tooling by adding comprehensive comments and JSDoc annotations to improve maintainability and understanding of the code. Specifically, it elaborates on package exports, conditions, and the getAliases function. Additionally, it completes the associated test file to ensure the getAliases function operates correctly in both monorepo and normal build scenarios.

Key changes:

  • Added detailed comments and JSDoc annotations to aliases.ts to explain the purpose and functionality of the code.
  • Completed the test file aliases.test.ts with test cases for normal builds and monorepo builds.
  • Mocked monorepo development aliases in the test to ensure accurate alias resolution.

Why these changes were introduced:

  • To prevent multiple context errors due to multiple instances of the library.
  • To improve code readability and maintainability.
  • To ensure comprehensive testing coverage for the alias resolution functionality.

What to review

  • Review the added comments and JSDoc annotations in aliases.ts for accuracy and clarity.
  • Review the test cases in aliases.test.ts to ensure they cover the expected scenarios and edge cases.
  • Ensure that the monorepo aliases are correctly mocked and resolved in the tests.

Testing

  • Added test cases for both normal builds and monorepo builds in aliases.test.ts.
  • Verified that the dynamic resolution logic in the tests mirrors the resolution logic used in getAliases.
  • Testing manually by packing and linking this in the admin studio.

Demo:

aliases.mp4

Notes for release

  • Adds aliases to internal vite config used by sanity build and sanity deploy to prevent missing React context issues.

@ricokahler ricokahler requested a review from a team as a code owner July 16, 2024 20:45
@ricokahler ricokahler requested review from rexxars and removed request for a team July 16, 2024 20:45
@vercel Vercel
Copy link

vercel bot commented Jul 16, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
page-building-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 19, 2024 4:07pm
performance-studio 🛑 Canceled (Inspect) 💬 Add feedback Jul 19, 2024 4:07pm
test-compiled-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 19, 2024 4:07pm
test-next-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 19, 2024 4:07pm
test-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 19, 2024 4:07pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Visit Preview Jul 19, 2024 4:07pm

@github-actions GitHub Actions
Copy link
Contributor

No changes to documentation

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 16, 2024 •
edited
Loading

Component Testing Report Updated Jul 19, 2024 3:52 PM (UTC)

File Status Duration Passed Skipped Failed
comments/CommentInput.spec.tsx ✅ Passed (Inspect) 44s 15 0 0
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 8s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ✅ Passed (Inspect) 31s 6 0 0
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx ✅ Passed (Inspect) 36s 11 7 0
formBuilder/inputs/PortableText/copyPaste/CopyPasteFields.spec.tsx ✅ Passed (Inspect) 0s 0 12 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 17s 6 0 0
formBuilder/inputs/PortableText/DisableFocusAndUnset.spec.tsx ✅ Passed (Inspect) 10s 3 0 0
formBuilder/inputs/PortableText/DragAndDrop.spec.tsx ✅ Passed (Inspect) 2m 33s 1 0 0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx ✅ Passed (Inspect) 43s 15 0 0
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 1m 44s 21 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ✅ Passed (Inspect) 1m 13s 18 0 0
formBuilder/inputs/PortableText/PresenceCursors.spec.tsx ✅ Passed (Inspect) 8s 3 9 0
formBuilder/inputs/PortableText/RangeDecoration.spec.tsx ✅ Passed (Inspect) 25s 9 0 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 18s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ❌ Failed (Inspect) 1m 23s 20 0 1
formBuilder/tree-editing/TreeEditing.spec.tsx ✅ Passed (Inspect) 1m 47s 30 0 0
formBuilder/tree-editing/TreeEditingNestedObjects.spec.tsx ✅ Passed (Inspect) 19s 3 0 0

@rexxars
Copy link
Member

rexxars commented Jul 16, 2024

Tests seem to be failing, will review when they pass!

@ricokahler ricokahler marked this pull request as draft July 16, 2024 21:06
@ricokahler ricokahler marked this pull request as ready for review July 16, 2024 21:38
@cngonzalez
Copy link
Member

These changes generally make sense to me and I'm sure work smoothly, but I'd feel more comfortable with someone more familiar with our Vite config having a look as well 🙇

@ricokahler ricokahler force-pushed the sdx-1471 branch 3 times, most recently from 8ca1c0d to 5711f84 Compare July 19, 2024 14:32
@cngonzalez cngonzalez self-requested a review July 19, 2024 15:00
cngonzalez
cngonzalez previously approved these changes Jul 19, 2024
View reviewed changes
@cngonzalez cngonzalez added this pull request to the merge queue Jul 19, 2024
Merged via the queue into next with commit 776f16b Jul 19, 2024
24 of 30 checks passed
@cngonzalez cngonzalez deleted the sdx-1471 branch July 19, 2024 15:37
stipsan pushed a commit that referenced this pull request Jul 22, 2024
@ricokahler @stipsan
feat(core): add aliases to mitigate context issues (#7172)
829b26d 
* feat(core): add aliases to mitigate context issues

* fix(core): use `find` `replacement` syntax for aliases

* docs: add TODO comment about improving test
stipsan pushed a commit that referenced this pull request Jul 22, 2024
@ricokahler @stipsan
feat(core): add aliases to mitigate context issues (#7172)
f1c2f86 
* feat(core): add aliases to mitigate context issues

* fix(core): use `find` `replacement` syntax for aliases

* docs: add TODO comment about improving test
@clinejj clinejj mentioned this pull request Jul 26, 2024
Closed
This was referenced Aug 10, 2024
Closed
Closed
cngonzalez pushed a commit that referenced this pull request Aug 20, 2024
@ricokahler @cngonzalez
feat(core): add aliases to mitigate context issues (#7172)
3243e7b 
* feat(core): add aliases to mitigate context issues

* fix(core): use `find` `replacement` syntax for aliases

* docs: add TODO comment about improving test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cngonzalez cngonzalez cngonzalez approved these changes

@rexxars rexxars Awaiting requested review from rexxars rexxars was automatically assigned from sanity-io/sdk

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ricokahler @rexxars @cngonzalez